-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Basis vectors #598
Basis vectors #598
Conversation
git checkout add_all_limits tests/test_axis_limits.py
Remove unneeded dependencies so that tests/test_axis_limits.py passes. Other changes were formatting changes forced by flake8 precommit.
@@ -917,6 +917,7 @@ def test_constrained_AL_lsq(): | |||
|
|||
@pytest.mark.slow | |||
@pytest.mark.regression | |||
@pytest.mark.xfail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #556 (comment)
@@ -665,7 +665,7 @@ def test_NAE_QIC_solve(): | |||
grid = LinearGrid(L=10, M=20, N=20, NFP=eq.NFP, sym=True, axis=False) | |||
iota = compress(grid, eq.compute("iota", grid=grid)["iota"], "rho") | |||
|
|||
np.testing.assert_allclose(iota[1], qsc.iota, atol=1e-5) | |||
np.testing.assert_allclose(iota[1], qsc.iota, atol=5e-4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #556 (comment)
This might be a good chance to add tests for the covariant basis vectors by comparing to finite differences of |
the finite differencing test seems to fail consistently for |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #598 +/- ##
==========================================
+ Coverage 94.30% 94.37% +0.06%
==========================================
Files 77 77
Lines 17696 17821 +125
==========================================
+ Hits 16689 16818 +129
+ Misses 1007 1003 -4
|
rtol = 1e-3 | ||
atol = 1e-3 | ||
num_zeta = 180 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we need to increase the tolerances, despite also increasing the finite difference resolution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably not, I just went for a consistent tolerance that worked for everything rather than trying to fine tune the tolerance for each different quantity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fine for rtol
, but in general could be dangerous for atol
since some derivates might be close to zero
Can't sort compute functions by quantity name anymore because there are multiple quantities with the same name. Have to sort by function name instead. Needed to update tests/test_axis_limits for the dependency check test.
@@ -173,94 +177,94 @@ def test_magnetic_field_derivatives(DummyStellarator): | |||
B_rr = np.apply_along_axis(my_convolve, 0, data["B"], FD_COEF_2_4) / drho**2 | |||
|
|||
np.testing.assert_allclose( | |||
data["B^theta_r"][3:-2], | |||
B_sup_theta_r[3:-2], | |||
data["B^theta_r"][4:-4], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why make these further from the bdry? because you increased nrho so want to keep it at the same rho values roughly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is same reason as #598 (comment)
Generalizes basis vectors to include toroidal stream function and adds their third derivatives.
This PR includes the second derivatives of the covariant basis vectors which were generated by Rory's code, and the following quantities that I coded:
@f0uriest Here is a diff of
_basis_vectors.py
from this branch with the sorted version_basis_vectors.py
from #568.https://diffy.org/diff/a1af98d05be0a.
git diff --no-index rc_toroidal_sorted_basis_vectors.txt _basis_vectors.py > output.diff
applied to rc_toroidal_sorted_basis_vectors.txt.